Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore circular "unlimited" vision for grid sights #5135

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Jan 13, 2025

Identify the Bug or Feature request

Fixes performance issue for #4980

Description of the Change

This restores the special case where a sight with no distance will produce a circular visible area that goes out to the map's Vision Distance setting. Grid shapes are not nearly performant enough yet to support that case, and it's not clear whether it's even desirable behaviour.

The key to the change is to hit the special case of range == 0 for Grid.getGridArea(). This case is now documented, and the override in GridlessGrid relies on it instead of duplicating the case logic.

Possible Drawbacks

None, this bring back long-standing behaviour.

Documentation Notes

N/A

Release Notes

  • Fixed grid sights without a distance to produce circular areas instead of grid shapes. Personal lights still follow the grid.

This change is Reviewable

@kwvanderlinde kwvanderlinde added bug performance A performance or quality of life improvement labels Jan 13, 2025
@kwvanderlinde kwvanderlinde self-assigned this Jan 13, 2025
Grid shapes are not nearly performant enough yet to support this case.

The special `range == 0` case is now documented, and the `GridlessGrid.getGridArea()` override relies on it now instead
of duplicating the case.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/4980-large-grid-vision branch from 62b78ea to 9f12d4e Compare January 13, 2025 19:52
@cwisniew cwisniew added this pull request to the merge queue Jan 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 14, 2025
@cwisniew cwisniew added this pull request to the merge queue Jan 14, 2025
Merged via the queue into develop with commit be8ee85 Jan 14, 2025
8 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/4980-large-grid-vision branch January 14, 2025 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug performance A performance or quality of life improvement
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

2 participants